Codexprepare seikinsettlement for deployment #2413
Conversation
…ting Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…tegration-tests Add conditional Slinky integration test
Potential fix for code scanning alert no. 422: Potentially unsafe quoting
…caching Improve CI workflows
…ng-issue Log estimate gas errors
…d-circlecciprouter-contracts-0x9h4c Add Keeper workflow and guard SeiKin settlement flows
…y-seikinsettlement-and-circlecciprouter-contracts
…d-circlecciprouter-contracts Add SeiKin settlement and Circle CCIP router contracts
…re-seikinsettlement-for-deployment
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
x/seinet/types/msgs.go
Outdated
| // GetSigners returns the message signers. | ||
| func (m *MsgCommitCovenant) GetSigners() []sdk.AccAddress { | ||
| addr, err := sdk.AccAddressFromBech32(m.Creator) | ||
| if err != nil { | ||
| return []sdk.AccAddress{} |
There was a problem hiding this comment.
[P0] Covenant commits can run with zero signatures
GetSigners returns an empty slice whenever Creator fails Bech32 parsing, and ValidateBasic later returns nil. A transaction can set an invalid creator, include no signatures, and still invoke SeiNetCommitCovenantSync, allowing anyone to commit a covenant or trigger royalty enforcement. The message should reject invalid addresses in ValidateBasic so that at least one signer is required.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create a Codex account and connect to github.
x/seinet/types/msgs.go
Outdated
| // GetSigners returns the message signers. | ||
| func (m *MsgUnlockHardwareKey) GetSigners() []sdk.AccAddress { | ||
| addr, err := sdk.AccAddressFromBech32(m.Creator) | ||
| if err != nil { | ||
| return []sdk.AccAddress{} |
There was a problem hiding this comment.
[P0] Hardware key unlock message skips authentication
Like MsgCommitCovenant, MsgUnlockHardwareKey silently returns no signers when the creator address is malformed and ValidateBasic performs no checks. An attacker can submit a transaction with a bogus creator and zero signatures to mark any key as unlocked. Ensure ValidateBasic validates the creator field (or return an error in GetSigners) so the message cannot execute without a valid signer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create a Codex account and connect to github.
x/seinet/types/msgs.go
Outdated
| // RegisterMsgServer is a no-op placeholder to satisfy interface in Configurator. | ||
| func RegisterMsgServer(s grpc.ServiceRegistrar, srv MsgServer) {} |
There was a problem hiding this comment.
[P1] Seinet message server never registered
RegisterMsgServer is an empty stub, yet AppModule.RegisterServices calls it to expose the module’s handlers. Because the registrar is never invoked, MsgCommitCovenant/MsgUnlockHardwareKey will be rejected with unknown service seinet.Msg and the module cannot be used over gRPC or standard transactions. Implement real registration (as other modules do) or remove the call.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create a Codex account and connect to github.
|
We truly appreciate your contribution and the time you’ve invested in this PR. |
Refactor MsgCommitCovenant and MsgUnlockHardwareKey to include basic validation for creator addresses and remove unused methods.
Updated workflow names and added Guardian Royalty Settlement job.
2 participants
Describe your changes and provide context
Testing performed to validate your change